[clang] Temporarily disable Darwin test for linking against libc++ on non-darwin systems #170912
+3
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… there i…" This reverts commit 190b8d0.
llvmbot
added
compiler-rt
clang:driver
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Kewen Meng (Kewen12) ChangesReverts llvm/llvm-project#170303 This PR breaks bots: https://lab.llvm.org/buildbot/#/builders/10/builds/18578. Revert to unblock. Full diff: https://github.com/llvm/llvm-project/pull/170912.diff 11 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 30c53389dc22f..fc3cd9030f71d 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2846,49 +2846,11 @@ void AppleMachO::AddCXXStdlibLibArgs(const ArgList &Args,
CXXStdlibType Type = GetCXXStdlibType(Args);
switch (Type) {
- case ToolChain::CST_Libcxx: {
- // On Darwin, we prioritize a libc++ located in the toolchain to a libc++
- // located in the sysroot. Unlike the driver for most other platforms, on
- // Darwin we do that by explicitly passing the library path to the linker
- // to avoid having to add the toolchain's `lib/` directory to the linker
- // search path, which would make other libraries findable as well.
- //
- // Prefering the toolchain library over the sysroot library matches the
- // behavior we have for headers, where we prefer headers in the toolchain
- // over headers in the sysroot if there are any. Note that it's important
- // for the header search path behavior to match the link-time search path
- // behavior to ensure that we link the program against a library that
- // matches the headers that were used to compile it.
- //
- // Otherwise, we end up compiling against some set of headers and then
- // linking against a different library (which, confusingly, shares the same
- // name) which may have been configured with different options, be at a
- // different version, etc.
- SmallString<128> InstallLib = llvm::sys::path::parent_path(getDriver().Dir);
- llvm::sys::path::append(InstallLib, "lib"); // <install>/lib
- auto Link = [&](StringRef Library) {
- SmallString<128> Shared(InstallLib);
- llvm::sys::path::append(Shared,
- SmallString<4>("lib") + Library + ".dylib");
- SmallString<128> Static(InstallLib);
- llvm::sys::path::append(Static, SmallString<4>("lib") + Library + ".a");
- SmallString<32> Relative("-l");
- Relative += Library;
-
- if (getVFS().exists(Shared)) {
- CmdArgs.push_back(Args.MakeArgString(Shared));
- } else if (getVFS().exists(Static)) {
- CmdArgs.push_back(Args.MakeArgString(Static));
- } else {
- CmdArgs.push_back(Args.MakeArgString(Relative));
- }
- };
-
- Link("c++");
+ case ToolChain::CST_Libcxx:
+ CmdArgs.push_back("-lc++");
if (Args.hasArg(options::OPT_fexperimental_library))
- Link("c++experimental");
+ CmdArgs.push_back("-lc++experimental");
break;
- }
case ToolChain::CST_Libstdcxx:
// Unfortunately, -lstdc++ doesn't always exist in the standard search path;
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++experimental.a
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/bin/.keep
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/include/c++/v1/.keep
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++.a
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a b/clang/test/Driver/Inputs/basic_darwin_toolchain_static/usr/lib/libc++experimental.a
deleted file mode 100644
index e69de29bb2d1d..0000000000000
diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp b/clang/test/Driver/darwin-header-search-libcxx.cpp
index e8985a4e22b2f..cc8ec9ceb89b3 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -92,7 +92,7 @@
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// Make sure that using -nostdinc, -nostdinc++ or -nostdlibinc will drop both the toolchain
+// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the toolchain
// C++ include path and the sysroot one.
//
// RUN: %clang -### %s -fsyntax-only 2>&1 \
@@ -116,7 +116,7 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -stdlib=platform \
// RUN: -nostdinc++ \
-// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
+// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
// CHECK-LIBCXX-NOSTDINCXX: "-cc1"
diff --git a/clang/test/Driver/darwin-link-libcxx.cpp b/clang/test/Driver/darwin-link-libcxx.cpp
deleted file mode 100644
index 1c4f31b257512..0000000000000
--- a/clang/test/Driver/darwin-link-libcxx.cpp
+++ /dev/null
@@ -1,81 +0,0 @@
-// UNSUPPORTED: system-windows
-
-// Tests to check that we link against the toolchain-provided libc++ built library when it is provided.
-// This is required to prefer the toolchain's libc++ over the system's libc++, which matches the behavior
-// we have for header search paths.
-
-// When libc++.dylib is NOT in the toolchain, we should use -lc++ and fall back to the libc++
-// in the sysroot.
-//
-// (1) Without -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN: --check-prefix=CHECK-1 %s
-// CHECK-1: "/usr/bin/ld"
-// CHECK-1: "-lc++"
-// CHECK-1-NOT: "[[TOOLCHAIN]]/usr/lib"
-//
-// (2) With -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN: -fexperimental-library \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN: --check-prefix=CHECK-2 %s
-// CHECK-2: "/usr/bin/ld"
-// CHECK-2: "-lc++" "-lc++experimental"
-// CHECK-2-NOT: "[[TOOLCHAIN]]/usr/lib"
-
-// When we have libc++.dylib in the toolchain, it should be used over the one in the sysroot.
-// There are a few cases worth testing.
-//
-// (1) Without -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN: --check-prefix=CHECK-3 %s
-// CHECK-3: "/usr/bin/ld"
-// CHECK-3: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
-// CHECK-3-NOT: "-lc++"
-//
-// (2) With -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
-// RUN: -fexperimental-library \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN: --check-prefix=CHECK-4 %s
-// CHECK-4: "/usr/bin/ld"
-// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++.dylib"
-// CHECK-4: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
-// CHECK-4-NOT: "-lc++"
-// CHECK-4-NOT: "-lc++experimental"
-
-// When we have libc++.a in the toolchain instead of libc++.dylib, it should be
-// used over the one in the sysroot.
-//
-// (1) Without -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
-// RUN: --check-prefix=CHECK-5 %s
-// CHECK-5: "/usr/bin/ld"
-// CHECK-5: "[[TOOLCHAIN]]/usr/lib/libc++.a"
-// CHECK-5-NOT: "-lc++"
-//
-// (2) With -fexperimental-library.
-// RUN: %clangxx -### %s 2>&1 \
-// RUN: --target=x86_64-apple-darwin \
-// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_static/usr/bin \
-// RUN: -fexperimental-library \
-// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_static \
-// RUN: --check-prefix=CHECK-6 %s
-// CHECK-6: "/usr/bin/ld"
-// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++.a"
-// CHECK-6: "[[TOOLCHAIN]]/usr/lib/libc++experimental.a"
-// CHECK-6-NOT: "-lc++"
-// CHECK-6-NOT: "-lc++experimental"
diff --git a/clang/test/Driver/experimental-library-flag.cpp b/clang/test/Driver/experimental-library-flag.cpp
index 0546a09d5518b..62b007516897e 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -18,8 +18,6 @@
// CHECK: -fexperimental-library
// Depending on the stdlib in use, we should (or not) pass -lc++experimental.
-// Note that we don't check for `-lc++experimental` specifically, since some targets
-// like Darwin pass the path to the library explicitly instead of using `-lx`.
-// CHECK-LIBCXX: c++experimental
-// CHECK-LIBSTDCXX-NOT: c++experimental
-// CHECK-NOSTDLIB-NOT: c++experimental
+// CHECK-LIBCXX: -lc++experimental
+// CHECK-LIBSTDCXX-NOT: -lc++experimental
+// CHECK-NOSTDLIB-NOT: -lc++experimental
diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 30a78b69515cc..084a7060e8d13 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -507,23 +507,8 @@ if(APPLE)
set(CMAKE_OSX_DEPLOYMENT_TARGET "")
set(DARWIN_COMMON_CFLAGS -stdlib=libc++)
-
- # This is tricky: We want to link against libc++, however the libc++ for the
- # architecture we're currently building may not have been built yet, since
- # compiler-rt on Darwin builds for all targets at once while libc++ builds for
- # a single target. Hence, we pass -nostdlib++ to disable the default mechanism
- # for finding libc++, and we pass -lc++ which will end up finding libc++ in the
- # SDK currently in use. That libc++ is the wrong libc++ to use if we're using
- # headers from a just-built libc++, but at least it contains all the architectures
- # we should be interested in.
- #
- # Fixing this properly would require removing the impedence mismatch between
- # the compiler-rt build on Darwin (which wants to build all architectures at
- # once) and the libc++ build, which produces a single architecture per CMake
- # invocation.
set(DARWIN_COMMON_LINK_FLAGS
-stdlib=libc++
- -nostdlib++
-lc++
-lc++abi)
|
zeroomega
approved these changes
Dec 5, 2025
Member
|
Please wait a few minutes before merging. |
Member
|
I would be tempted to disable the test on non-Darwin instead, while I figure out how to best re-enable it. All of the failures seem to be on Linux builders which happen to be using |
…f there i…" This reverts commit 12a532c.
Member
|
LMK if you're fine with this way of fixing the issue (temporarily) instead: it will allow me to investigate while unbreaking the bots, and that way we get additional feedback on this change since we don't revert entirely. |
Member
|
I think I have a clean fix already, but I'm OK with reverting this partially per my suggestion and then removing the |
Contributor
Author
Hi @ldionne, Thanks for the fix! Shall we proceed with this revert PR? |
Member
|
Yeah, I'm fine with that. I will edit the commit message. |
ldionne
changed the title
Member
|
This will auto-merge once CI is finished running (shouldn't be very long). |
Contributor
Author
|
cool, Thanks! |
ldionne
deleted the
revert-170303-review/clang-link-against-built-libcxx-darwin
branch
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/28491 Here is the relevant piece of the build log for the reference |
Member
|
^ This failure doesn't seem related to my change. In fact, it looks like something spurious since we're hitting a resource limit. |
ldionne
added a commit
that referenced
this pull request
Dec 5, 2025
This patch reverts the change that made clang prefer the toolchain libc++.dylib when there is one (#170303), and the subsequent test workaround we landed to fix bots (#170912). We are seeing some failure on macOS LLDB bots that need to be investigated, and that will require more time than I can spare before the end of today. This reverts commits bad1a88 and 190b8d0.
Member
|
I ended up doing a full revert due to other failures in LLDB tests that will take longer to address. |
ronlieb
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Dec 5, 2025
This patch reverts the change that made clang prefer the toolchain libc++.dylib when there is one (llvm#170303), and the subsequent test workaround we landed to fix bots (llvm#170912). We are seeing some failure on macOS LLDB bots that need to be investigated, and that will require more time than I can spare before the end of today. This reverts commits bad1a88 and 190b8d0.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disable the test added in #170303, which breaks bots that don't use ld
as their linker. This is a temporary and narrow disablement of the test
until we can make it more general again, to get the bots green.